spec/fixtures/vcr_cassettes/Mailtrap_Template/_get/when_template_does_not_exist/raises_not_found_error.yml (1)

1-327: Slim down the VCR fixture by filtering out dynamic and unnecessary headers
Duplicate of the comment for the _list cassette above.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.yml (1)

1-164: Slim down the VCR fixture by filtering out dynamic and unnecessary headers
Duplicate of the comment for the _list cassette above.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_update/with_hash_request/returns_an_EmailTemplate_object.yml (1)

1-333: Slim down the VCR fixture by filtering out dynamic and unnecessary headers
Duplicate of the comment for the _list cassette above.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_create/with_hash_request/maps_response_data_to_EmailTemplate_object.yml (1)

1-168: Slim down the VCR fixture by filtering out dynamic and unnecessary headers
Duplicate of the comment for the _list cassette above.

CHANGELOG.md (1)

1-3: Changelog entry omits major user-facing additions

The 2.3.1 note only mentions EmailTemplates, but this PR also delivers the new Contacts and ContactLists APIs. Omitting them makes the changelog an unreliable source for upgrade information.

-## [2.3.1] - 2025-06-17
-- Add EmailTemplates API functionality
+## [2.3.1] - 2025-06-17
+- Add EmailTemplates API functionality
+- Add Contacts API (`Mailtrap::Contact`, DTOs)
+- Add ContactLists API (`Mailtrap::ContactList`, DTOs)

lib/mailtrap.rb (1)

7-9: Consider lazy-loading to keep boot-time low

Unconditionally requiring three new files forces all Contact / ContactList / Template code (and their dependencies) to be loaded even in apps that never use them. Using autoload (or Zeitwerk if available) defers cost until first use.

-module Mailtrap; end
+module Mailtrap
+  autoload :Template,     'mailtrap/template'
+  autoload :Contact,      'mailtrap/contact'
+  autoload :ContactList,  'mailtrap/contact_list'
+end

spec/mailtrap/client_spec.rb (1)

217-219: Error message expectation contains a stray comma

The message being asserted ('client error:, 🫖') has an extra comma after the colon, which looks accidental and makes the text clunky.

-        expect { send_mail }.to raise_error(Mailtrap::Error, 'client error:, 🫖')
+        expect { send_mail }.to raise_error(Mailtrap::Error, 'client error: 🫖')

spec/spec_helper.rb (1)

17-19: Mask account_id in query-params as well

The current substitutions only touch the path and JSON body. If the account_id ever appears in a query string (e.g., /accounts/123/emails?account_id=123) it will leak into the cassette.

-    interaction.request.uri.gsub!(%r{/accounts/\d+/}, '/accounts/1111111/')
+    interaction.request.uri.gsub!(%r{/accounts/\d+/}, '/accounts/1111111/')
+    interaction.request.uri.gsub!(/account_id=\d+/, 'account_id=1111111')

examples/email_template.rb (2)

1-3: Prefer ENV variables over hard-coding secrets

Hard-coding an API key in example code makes it too easy for newcomers to accidentally commit real credentials.
Use an environment variable so that the snippet is copy-paste-safe:

-client = Mailtrap::Client.new(api_key: 'your-api-key')
+client = Mailtrap::Client.new(api_key: ENV.fetch('MAILTRAP_API_KEY'))

Likewise consider ENV['MAILTRAP_ACCOUNT_ID'] (or similar) for the numeric ID.

---

21-32: Inconsistent accessor style – pick obj.id over hash-style obj[:id]

created, updated, etc. are EmailTemplate objects. Switching between created[:id] and
created.id (or found[:name] vs found.name) is confusing and leaks internal implementation
details ([] most likely delegates to @data). Sticking to reader methods keeps the public
surface minimal and discoverable.

-puts "Created Template: #{created[:id]}"
+puts "Created Template: #{created.id}"
…
-found = templates.get(created[:id])
+found = templates.get(created.id)
…
-puts "Updated Template Name: #{updated[:name]}"
+puts "Updated Template Name: #{updated.name}"

examples/contact.rb (2)

4-5: Same secret-handling remark as above

-client = Mailtrap::Client.new(api_key: 'your-api-key')
+client = Mailtrap::Client.new(api_key: ENV.fetch('MAILTRAP_API_KEY'))

---

22-24: Large puts dumps harm readability of example output

Dumping whole arrays / objects via puts produces noisy, often unreadable JSON. Consider pretty
printing or at least outputting counts / ids to keep the example concise.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_list/maps_response_data_to_EmailTemplate_objects.yml (1)

21-60: Filter out dynamic response headers
This cassette includes many volatile headers (Date, Cf-Ray, X-Request-Id, ETag, X-Runtime, etc.) that can break tests on reruns. Consider configuring VCR to strip or stub these headers to improve fixture stability.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_create/maps_response_data_to_EmailTemplate_object.yml (1)

25-63: Remove or filter noisy response headers
The response contains a long list of transient security and tracing headers (e.g., Cache-Control, Content-Security-Policy, Cf-Ray, X-Request-Id). Trimming these in fixtures or filtering via VCR will make your tests more maintainable.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_create/returns_an_EmailTemplate_object.yml (1)

56-63: Strip volatile values from headers
Response headers like Etag, Strict-Transport-Security, and dynamic CSP strings add noise. Filter or remove them for cleaner fixtures.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_delete/when_template_does_not_exist/raises_not_found_error.yml (1)

185-193: Simplify error response headers
The 404 response body is concise, but you still include dynamic headers (Cf-Ray, Cache-Control, etc.). Consider filtering these out to avoid brittle tests.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_get/returns_an_EmailTemplate_object.yml (1)

167-175: Filter dynamic values in GET response
The GET response includes transient headers (ETag, Cache-Control, Cf-Ray, etc.). Removing or filtering them will help keep the fixture lean and stable.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_update/with_hash_request/maps_response_data_to_EmailTemplate_object.yml (1)

1-332: Consider filtering dynamic response headers to reduce fixture noise.

The cassette correctly captures the POST and PATCH interactions and uses placeholders for sensitive values. However, repeated dynamic headers (e.g., Date, Etag, Cf-Ray, X-Request-Id) inflate the fixture and may change frequently, leading to brittle tests.
Recommend adding VCR header filtering or using filter_sensitive_data in spec_helper.rb to strip or normalize these headers.

spec/mailtrap/contact_list_spec.rb (2)

1-1: Add the frozen string literal magic comment

The rest of the spec suite uses the Ruby 2.3+ immutable string convention. Adding the directive keeps style-checks green and avoids needless diff-noise.

+# frozen_string_literal: true
 RSpec.describe Mailtrap::ContactList do

---

17-29: Trim long examples or enable aggregate_failures

RuboCop flags these as exceeding the 5-line guideline. Either:

1. Split assertions into separate it blocks, or
2. Add aggregate_failures to keep them concise while silencing the cop.

Low-priority, but keeping specs terse improves readability.

spec/mailtrap/contact_spec.rb (3)

215-217: Remove stray debugging output

pp response leaks noisy output to the test run and CI logs.

-      pp response

Delete the line or replace it with a purposeful assertion if you need to inspect the object.

---

17-31: Avoid large duplicated JSON payloads – extract to a helper

The same expected_response hash (or very similar) is repeated in multiple contexts.
Extract to a helper/factory to DRY the spec and make future payload changes cheaper:

def contact_payload(overrides = {})
  {
    'data' => {
      'id' => contact_id,
      'email' => email,
      'created_at' => 1_748_163_401_202,
      'updated_at' => 1_748_163_401_202,
      'list_ids' => [1, 2],
      'status' => 'subscribed',
      'fields' => { 'first_name' => 'John', 'last_name' => nil }
    }
  }.deep_merge(overrides)
end

Then use contact_payload in every stub.

Also applies to: 51-66, 129-144

---

6-7: Hard-coded API key/account ID in specs – prefer ENV placeholders

Even in test code, static secrets trigger scanners and may be copy-pasted elsewhere.

-let(:client) { described_class.new('1111111', Mailtrap::Client.new(api_key: 'correct-api-key')) }
+let(:client) { described_class.new('1111111', Mailtrap::Client.new(api_key: ENV.fetch('MAILTRAP_TEST_API_KEY', 'dummy'))) }

Keeps tooling quiet and avoids accidental leakage.

lib/mailtrap/template.rb (1)

46-48: MAX_LENGTH and MAX_BODY_LENGTH are dead code

The constants are declared but never referenced.
Either implement input validation (raise ArgumentError when exceeded) or drop the constants to avoid false “intention” signals.

-    MAX_LENGTH = 255
-    MAX_BODY_LENGTH = 10_000_000

spec/mailtrap/template_spec.rb (1)

40-50: Creating real templates inside let! couples tests and slows suites

let! executes before every example in the group, creating N identical templates (and recording N cassettes).
Prefer before(:all) / around(:all) or a VCR cassette recorded once, then reuse the data to keep the suite fast and deterministic.

before(:all) do
  @created_template = template.create(...)
end
let(:template_id) { @created_template.id }

Remember to clean up in an after(:all) block if the endpoint has side-effects.

lib/mailtrap/client.rb (3)

34-36: Parameter naming drift

The rest of the client uses api_host/api_port; introducing a second host field (general_api_host) complicates configuration and increases the chance of mismatching hosts. Consider re-using the existing api_host for all non-send traffic or clearly documenting when each one is used.

Also applies to: 49-50

---

55-57: Build URIs with HTTPS, not HTTP

All requests are forced to use_ssl = true, but the URI objects are created with URI::HTTP.build. While Ruby will still perform TLS, logging, redirects, and HSTS logic will report the wrong scheme. Switch to URI::HTTPS.build (or URI("https://…")) for accuracy and clarity.

Also applies to: 59-66, 67-75, 76-83, 85-91

---

111-117: Use the port from the URI

Net::HTTP.new(uri.host, @api_port) ignores the port embedded in the URI (which URI::HTTPS.build would supply). Use uri.port to stay consistent with the constructed URI.

-http_client = Net::HTTP.new(uri.host, @api_port)
+http_client = Net::HTTP.new(uri.host, uri.port)

spec/fixtures/vcr_cassettes/Mailtrap_Template/_list/returns_an_array_of_EmailTemplate_objects.yml (1)

1-319: Slim down the VCR fixture by filtering out dynamic and unnecessary headers

This cassette includes dozens of dynamic or security-related headers (e.g., Cf-Ray, ETag, Cache-Control, Content-Security-Policy) that bloat its size and make tests brittle. Please configure VCR to strip all but essential headers (for instance only Content-Type and X-Mailtrap-Version) and match requests on just method and URI. Consider adding to spec/spec_helper.rb:

VCR.configure do |config|
  config.before_record do |interaction|
    # Keep only essential response headers
    interaction.response.headers.select! { |key, _| ['Content-Type', 'X-Mailtrap-Version'] }
  end
  config.default_cassette_options = { match_requests_on: [:method, :uri] }
end

spec/fixtures/vcr_cassettes/Mailtrap_Template/_update/maps_response_data_to_EmailTemplate_object.yml (2)

1-167: Consolidate and trim verbose headers for the create interaction
The http_interactions[0] entry includes a large set of low-value headers (e.g., Content-Security-Policy, X-Frame-Options, Cf-Ray, etc.) that bloat the fixture and make it brittle. Please filter out or remove non-essential headers and rely on VCR’s validation of core fields (method, URI, status code) to stabilize tests. Also ensure that the account ID (1111111) and the bearer token are replaced with VCR placeholders (<ACCOUNT_ID>, <BEARER_TOKEN>) to avoid leaking sensitive data.

---

168-331: Apply same header cleanup and placeholder normalization for the update interaction
Repeat the trimming of unnecessary headers in the second interaction (PATCH /accounts/1111111/email_templates/39715). Confirm that dynamic values—especially the account ID—are filtered to <ACCOUNT_ID> and that only stable headers are retained in the cassette.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_update/when_template_does_not_exist/raises_not_found_error.yml (2)

1-167: Normalize sensitive data and remove noise from the create step
The initial POST interaction carries the same verbose headers and a hard-coded account ID. Please consolidate the fixture by stripping non-critical headers and replacing 1111111 with <ACCOUNT_ID>, ensuring the bearer token remains a placeholder (<BEARER_TOKEN>).

---

168-328: Validate 404 scenario and clean up headers
The error case for PATCH /accounts/1111111/email_templates/999999 correctly returns a 404 payload. To improve readability and maintainability, trim unnecessary headers, normalize the account ID to <ACCOUNT_ID>, and verify that your client’s test suite expects this exact {"error":"Not Found"} body.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

lib/mailtrap/contact.rb (3)

87-89: URL encoding missing for contact identifier.

The contact_id parameter can be an email address containing special characters like @ and +. Without URL encoding, this will cause test failures (stubs won't match) and API errors.

   def get(contact_id)
-    response = @client.get("#{base_path}/#{contact_id}")
+    encoded_id = CGI.escape(contact_id.to_s)
+    response = @client.get("#{base_path}/#{encoded_id}")
     build_entity(response[:data], Contact)
   end

Don't forget to add require 'cgi' at the top of the file.

---

112-118: Apply consistent URL encoding in update method.

Same URL encoding issue applies to the update method.

   def update(contact_id, request)
+    encoded_id = CGI.escape(contact_id.to_s)
     response = @client.patch(
-      "#{base_path}/#{contact_id}",
+      "#{base_path}/#{encoded_id}",
       { contact: prepare_request(request, ContactUpdateRequest) }
     )
     build_entity(response, ContactUpdateResponse)
   end

---

127-129: Apply consistent URL encoding in delete method.

Same URL encoding issue applies to the delete method.

   def delete(contact_id)
-    @client.delete("#{base_path}/#{contact_id}")
+    encoded_id = CGI.escape(contact_id.to_s)
+    @client.delete("#{base_path}/#{encoded_id}")
   end

lib/mailtrap/api.rb (1)

1-1: Add frozen string literal comment for performance.

The static analysis correctly identifies the missing frozen string literal comment.

+# frozen_string_literal: true
+
 module Mailtrap

spec/mailtrap/contacts_api_spec.rb (1)

212-212: Remove debug statement.

The pp response statement appears to be leftover debug code and should be removed.

       response = client.update(contact_id, update_data)
-      pp response

spec/mailtrap/contact_lists_api_spec.rb (1)

1-1: Add frozen string literal comment.

Missing frozen string literal comment for consistency with other test files.

+# frozen_string_literal: true
+
 RSpec.describe Mailtrap::ContactListsAPI do

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

lib/mailtrap/contacts_api.rb (1)

1-72: Good implementation with minor inconsistency in response handling.

The ContactsAPI class is well-implemented with:

• Proper inheritance from BaseAPI
• Comprehensive CRUD operations
• Good parameter validation using predefined constants
• Excellent documentation with YARD syntax
• Support for both UUID and email identifiers

However, there's an inconsistency in response handling:

 def get(contact_id)
   response = client.get("#{base_path}/#{contact_id}")
-  build_entity(response[:data], Contact)
+  build_entity(response, Contact)
 end

The get method accesses response[:data] while create and update methods expect the response structure to match the entity. Consider standardizing the response handling pattern across all methods or document why different approaches are needed.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

lib/mailtrap/contacts_api.rb (1)

59-70: Well-implemented list management methods

The dedicated add_to_lists and remove_from_lists methods follow the reviewer's previous suggestion and provide a clean API for managing contact lists.

Also applies to: 72-83

lib/mailtrap/contacts_api.rb (1)

49-57: Consider using base_update for consistency

The update method manually implements the update logic instead of using base_update like other methods. This breaks the pattern established by the BaseAPI module.

Since the method needs to return ContactUpdateResponse instead of Contact, you could override the response handling:

 def update(contact_id, options)
-  validate_options!(options, %i[email fields unsubscribed])
-
-  response = client.patch(
-    "#{base_path}/#{contact_id}",
-    { contact: options }
-  )
-  build_entity(response, ContactUpdateResponse)
+  base_update(contact_id, options, %i[email fields unsubscribed])
 end
+
+# Override to return ContactUpdateResponse for update
+def handle_update_response(response)
+  build_entity(response, ContactUpdateResponse)
+end

Then modify BaseAPI to call handle_update_response if it exists.

lib/mailtrap/base_api.rb (1)

65-69: Consider making response handling more flexible

The current design assumes all responses use the same response_class, but as seen in ContactsAPI, the update method needs to return a different response type.

Consider adding method-specific response handling:

 def base_update(id, options, supported_options_override = supported_options)
   validate_options!(options, supported_options_override)
   response = client.patch("#{base_path}/#{id}", wrap_request(options))
-  handle_response(response)
+  handle_update_response(response)
 end

+def handle_update_response(response)
+  handle_response(response)
+end

This would allow subclasses to override response handling for specific operations while maintaining the default behavior.

Also applies to: 71-75, 86-88

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

lib/mailtrap/contacts_api.rb (1)

9-10: Address the inconsistent supported options configuration

This issue was previously identified but remains unresolved. The supported_options declares list_ids but the update method validates against different options (email, fields, unsubscribed). The list_ids functionality is correctly handled by dedicated methods add_to_lists and remove_from_lists.

Remove list_ids from the supported options since list management is handled by dedicated methods:

-supported_options %i[email fields list_ids]
+supported_options %i[email fields]

lib/mailtrap/contacts_api.rb (1)

73-78: Consider extracting the base_update override

The base_update method override with supported_options_override parameter is a good approach for handling different validation requirements. However, this pattern might be useful in other API classes as well.

Consider moving this pattern to the BaseAPI module to make it available for other API classes that need similar functionality.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

lib/mailtrap/contacts_api.rb (2)

16-18: URL encoding is still missing for contact_id parameter

The contact_id parameter can be an email address containing special characters like @, +, etc. Without URL encoding, this will cause test failures and API errors.

Apply URL encoding to the contact_id parameter:

def get(contact_id)
-  base_get(contact_id)
+  encoded_id = CGI.escape(contact_id.to_s)
+  base_get(encoded_id)
end

The same encoding should be applied to the delete method and any other methods that use contact_id in URL paths.

---

73-78: Custom base_update method needs URL encoding

The custom base_update method directly interpolates the id parameter into the URL path without encoding, which will cause issues with email addresses containing special characters.

Apply URL encoding to the id parameter:

def base_update(id, options, supported_options_override = supported_options)
  validate_options!(options, supported_options_override)

-  response = client.patch("#{base_path}/#{id}", wrap_request(options))
+  encoded_id = CGI.escape(id.to_s)
+  response = client.patch("#{base_path}/#{encoded_id}", wrap_request(options))
  build_entity(response[:data].merge(action: response[:action]), Contact)
end

lib/mailtrap/base_api.rb (1)

29-36: Address account_id type inconsistency

The documentation states account_id should be an Integer, but ENV.fetch returns a String. This type inconsistency should be resolved.

Either convert to Integer in the initializer:

def initialize(account_id = ENV.fetch('MAILTRAP_ACCOUNT_ID'), client = Mailtrap::Client.new)
  raise ArgumentError, 'account_id is required' if account_id.nil?

-  @account_id = account_id
+  @account_id = Integer(account_id)
  @client = client
end

Or update the documentation to reflect that both types are accepted:

-# @param account_id [Integer] The account ID
+# @param account_id [String, Integer] The account ID (numeric value)

spec/mailtrap/contacts_api_spec.rb (4)

155-155: Fix RuboCop formatting: Use 'be' for boolean comparison

-expect(response.newly_created?).to eq(true)
+expect(response.newly_created?).to be(true)

---

212-219: Fix indentation and boolean comparison

      expect(response).to have_attributes(
-          id: contact_id,
-          fields: include(
-            last_name: 'Smith'
-          ),
-          action: 'updated'
-        )
-      expect(response.newly_created?).to eq(false)
+        id: contact_id,
+        fields: include(
+          last_name: 'Smith'
+        ),
+        action: 'updated'
+      )
+      expect(response.newly_created?).to be(false)

---

234-237: Fix indentation and boolean comparison

      expect(response).to have_attributes(
-          email:
-      )
-      expect(response.newly_created?).to eq(false)
+        email:
+      )
+      expect(response.newly_created?).to be(false)

---

292-296: Fix indentation

      expect(response).to have_attributes(
-          id: contact_id,
-          list_ids: include(1, 2, 3, 4, 5),
-          action: 'updated'
-      )
+        id: contact_id,
+        list_ids: include(1, 2, 3, 4, 5),
+        action: 'updated'
+      )